Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: do not hardcode Net::SSH auth_methods #440

Merged
merged 3 commits into from
Sep 11, 2023
Merged

fix: do not hardcode Net::SSH auth_methods #440

merged 3 commits into from
Sep 11, 2023

Conversation

gf3
Copy link
Contributor

@gf3 gf3 commented Sep 1, 2023

Hardcoding the Net::SSH auth_methods option to ["publickey"] prevents alternative authentication methods such as solutions like Tailscale which obviate the need to store and share private keys. The default auth_methods are “publickey”, “hostbased”, “password”, and “keyboard-interactive”—and they are tried in that order. Which means this change will still attempt to use the "publickey" method first and will not break backwards compatibility.

@djmb
Copy link
Collaborator

djmb commented Sep 6, 2023

Thanks for this @gf3!

There's a couple of test failures - would you be able to fix those up? 🙏

2023-09-06T07:58:27.0468706Z Failure:
2023-09-06T07:58:27.0469031Z ConfigurationTest#test_to_h [/home/runner/work/kamal/kamal/test/configuration_test.rb:268]:
2023-09-06T07:58:27.0469598Z --- expected
2023-09-06T07:58:27.0469875Z +++ actual
2023-09-06T07:58:27.0470669Z @@ -1 +1 @@
2023-09-06T07:58:27.0472338Z -{:roles=>["web"], :hosts=>["1.1.1.1", "1.1.1.2"], :primary_host=>"1.1.1.1", :version=>"missing", :repository=>"dhh/app", :absolute_image=>"dhh/app:missing", :service_with_version=>"app-missing", :env_args=>["-e", "REDIS_URL=\"redis://x/y\""], :ssh_options=>{:user=>"root", :auth_methods=>["publickey"], :log_level=>:fatal, :keepalive=>true, :keepalive_interval=>30}, :sshkit=>{}, :volume_args=>["--volume", "/local/path:/container/path"], :builder=>{}, :logging=>["--log-opt", "max-size=\"10m\""], :healthcheck=>{"path"=>"/up", "port"=>3000, "max_attempts"=>7}}
2023-09-06T07:58:27.0474431Z +{:roles=>["web"], :hosts=>["1.1.1.1", "1.1.1.2"], :primary_host=>"1.1.1.1", :version=>"missing", :repository=>"dhh/app", :absolute_image=>"dhh/app:missing", :service_with_version=>"app-missing", :env_args=>["-e", "REDIS_URL=\"redis://x/y\""], :volume_args=>["--volume", "/local/path:/container/path"], :ssh_options=>{:user=>"root", :keepalive=>true, :keepalive_interval=>30, :log_level=>:fatal}, :sshkit=>{}, :builder=>{}, :logging=>["--log-opt", "max-size=\"10m\""], :healthcheck=>{"path"=>"/up", "port"=>3000, "max_attempts"=>7}}
2023-09-06T07:58:27.0475165Z 
2023-09-06T07:58:27.0475763Z 
2023-09-06T07:58:27.0476147Z bin/test home/runner/work/kamal/kamal/test/configuration_test.rb:251

2023-09-06T08:00:10.5299217Z Failure:
2023-09-06T08:00:10.5299534Z MainTest#test_config [/home/runner/work/kamal/kamal/test/integration/main_test.rb:54]:
2023-09-06T08:00:10.5299943Z --- expected
2023-09-06T08:00:10.5300308Z +++ actual
2023-09-06T08:00:10.5300523Z @@ -1 +1 @@
2023-09-06T08:00:10.5300966Z -{:user=>"root", :auth_methods=>["publickey"], :keepalive=>true, :keepalive_interval=>30, :log_level=>:fatal}
2023-09-06T08:00:10.5301377Z +{:user=>"root", :keepalive=>true, :keepalive_interval=>30, :log_level=>:fatal}
2023-09-06T08:00:10.5301584Z 
2023-09-06T08:00:10.5301591Z 
2023-09-06T08:00:10.5301758Z bin/test home/runner/work/kamal/kamal/test/integration/main_test.rb:41

@gf3
Copy link
Contributor Author

gf3 commented Sep 6, 2023

@djmb updated!

@djmb djmb merged commit 9d35793 into basecamp:main Sep 11, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gf3 @djmb